-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(puppeteer): Upgrade to axe-core 4.3 #327
Conversation
* In this project, most async calls needs to block execution, such as | ||
* the driver.switchTo().frame() calls. These must run in the correct order | ||
* to avoid stale element references and infinite loops testing the same frame. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, copy from WDJS. Will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love all the deleted files. I'm fine leaving the hack in-place for this pr as it's just a typescript hack that isn't needed anymore.
await new AxePuppeteer(page, axeSource).analyze(); | ||
assert(evalSpy.calledWith(axeSource)); | ||
}); | ||
// TODO: Defaults to using the bundled axe-core source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to worry about this TODO in this pr?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was copied from the other source... probably shouldn't be here. I'll just take it out.
// HACK: work around axe-core (incorrectly) requiring this to be | ||
// a function (see https://github.com/dequelabs/axe-core/issues/974). | ||
(config.checks as any)[0].evaluate = 'function () { return false }'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should no longer be required as evaluate can be a string in the typings
// HACK: work around axe-core (incorrectly) requiring this to be | |
// a function (see https://github.com/dequelabs/axe-core/issues/974). | |
(config.checks as any)[0].evaluate = 'function () { return false }'; | |
config.checks[0].evaluate = 'function () { return false }'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied this from the previous implementation, but yeah it can be removed.
No description provided.